Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally write out executed notebook in jupyter-execute #307

Merged
merged 4 commits into from
Mar 12, 2024

Conversation

wpk-nist-gov
Copy link
Contributor

Fixes #306.

Now actually write out the executed notebook to the same path as the input.

wpk added 2 commits February 29, 2024 15:26
Fixes jupyter#306.

Now actually write out the executed notebook to the same path
as the input.
This commit adds two options to optionally save the executed notebook.

* `--inplace`:  Save the executed notebook to the input notebook path
* `--output`: Save the executed notebook to `output`.  This option
  can take a pattern like `{notebook_name}-new`, where `notebook_name` is
  the name of the input notebook without extension `.ipynb`.  Also, the
  output location is always relative to the input notebook location.
Copy link
Contributor

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@blink1073 blink1073 added the enhancement New feature or request label Mar 12, 2024
@blink1073 blink1073 changed the title fix: Actually write out executed notebook in jupyter-execute Optionally write out executed notebook in jupyter-execute Mar 12, 2024
@blink1073 blink1073 enabled auto-merge (squash) March 12, 2024 15:42
@blink1073
Copy link
Contributor

The failure seems relevant, though I'm not sure what is triggering it about the line:

nbclient/cli.py:200: in run_notebook
    with input_path.open() as f:
...
E       DeprecationWarning: pathlib.Path.__enter__() is deprecated and scheduled for removal in Python 3.13; Path objects as a context manager is a no-op

@wpk-nist-gov
Copy link
Contributor Author

wpk-nist-gov commented Mar 12, 2024

I think the error is due to how I mocked things. Working on it...

Yup, that was due to mocking pathlib.Path. Should work now..

auto-merge was automatically disabled March 12, 2024 17:43

Head branch was pushed to by a user without write access

@wpk-nist-gov
Copy link
Contributor Author

No clue why CI is/was failing for macos 3.10. I merged in changes from main. Hopefully that fixes it...

@blink1073
Copy link
Contributor

No clue why CI is/was failing for macos 3.10.

The macos tests have been flaky for some time now.

@blink1073
Copy link
Contributor

Thanks again!

@blink1073 blink1073 merged commit 3286ae0 into jupyter:main Mar 12, 2024
23 checks passed
@wpk-nist-gov
Copy link
Contributor Author

Thanks again!

My pleasure!

@wpk-nist-gov wpk-nist-gov deleted the feature/write-cli-output branch March 12, 2024 20:36
Comment on lines -139 to +180
name = notebook_path.replace(".ipynb", "")
input_path = Path(notebook_path).with_suffix(".ipynb")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wpk-nist-gov Pardon my potential ignorance, but doesn't this change the input behavior?
The code was initially stripping the suffix then adding it, but this change adds the suffix indiscriminately. Seems if I pass a path to a notebook, the ipynb suffix will be duplicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with_suffix respects if the path already has the extension. For example:

>>> from pathlib import Path
# with suffix already there, don't replace
>>> Path("a/b/c.ipynb").with_suffix(".ipynb")
PosixPath('a/b/c.ipynb')
# without suffix, add it
>>> Path("a/b/c").with_suffix(".ipynb")
PosixPath('a/b/c.ipynb')

The previous behavior was to replace, then add the suffix:

name = notebook_path.replace(".ipynb", "")

input_path = f"{name}.ipynb"

These should be mostly equivalent...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Calling jupyter-execute runs the notebook but doesn't save it
3 participants